Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: improve css render performance #1476

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

perf: improve css render performance #1476

wants to merge 2 commits into from

Conversation

BioPhoton
Copy link

@BioPhoton BioPhoton commented Jan 11, 2024 •
edited
Loading

The Problem

The page has hight render time especially for recalculate styles layout also paint is not optimal.

Browser Render Pipeline improvements nuxt

Steps to reproduce

  1. Visite https://nuxt.com/modules as it is a good candidate to quickly test it.
  2. As I'm on a MacBook Pro M2 Max I used 4x throttling to emulate more commonly used devices
  3. Hit the toggle dark mode button a couple of times in a row
Screenshot 2024-01-11 at 13 33 03

You should see a heavy delay in responsive ness due to heavy recalc and layouting.

Measurement

Desktop unthrottled

Screenshot 2024-01-11 at 13 40 21

Desktop 4x throttled

Screenshot 2024-01-11 at 13 40 04

Mobile unthrottled

Screenshot 2024-01-11 at 13 41 45

Mobile 4x throttled
Screenshot 2024-01-11 at 13 41 30

Solution

Use CSS techniques that improve rendering

Poc

To get quick insight how the changes improve rendering I used source overwrites in the Chrome DevTools.

Here the used snippet:

/* Contain and content visibility */
img, svg, span[class^="i-ph"] {
 contain:content;
}

header > div {
 contain:size;

}


#smooth > div:nth-child(2) div.grid > div,
footer h3, footer li,
span[class^="i-ph"] {
    content-visibility: auto;
}


#smooth > div:nth-child(2) div.grid > div {
    intrinsic-sice: 238px;
}

footer h3, footer li {
    intrinsic-sice: 18px;
}
span[class^="i-ph"] {
 intrinsic-sice: 20px;
}

/* Mobile/Tablet */
#smooth > div:first-child {
  display: none;
}

This PR includes

style adoptions as well as adding img lazy loading in the following files:

  • index
  • components:
    • AppFooter
    • AppHeader
  • blog
    • slug
    • index
  • enterprise
    • job
    • sponsors
  • modules
    • slug
    • index

Measurement before/after 4x throttled

Mini map comparison
Screenshot 2024-01-11 at 14 00 09

Mini map comparison inc CWV plugin
Screenshot 2024-01-11 at 13 59 56

Im happy to adjust/move code around as I am not really familiar with the code base and assume the location of the code could be improved...

All theory on the improvements can be found here: https://github.com/push-based/css-contain-and-content-visibility-research

manniL
manniL reviewed Jan 11, 2024
View reviewed changes
Copy link
Member

@manniL manniL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great findings 👍

pages/index.vue Outdated Show resolved Hide resolved
pages/index.vue
@@ -16,7 +16,41 @@ useSeoMeta({
twitterImage: joinURL(site.url, '/new-social.jpg')
})
</script>

<style>
.content-visibility-auto {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably be nice to define these Tailwind utils "globally" 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! where can i put them??

I'm a beginner vue coder just so u be humble with my ugly code :D

pages/modules/index.vue
@@ -45,6 +45,29 @@ defineShortcuts({

const { copy } = useCopyToClipboard()
</script>
<style>
#module-categories {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose these could be moved to tailwind-equivalents

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you give me more info what this means I'll do it right now

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually global classes are added as a plugin in Tailwind. As a matter of fact, the docs feature an example using content-visibility here: https://tailwindcss.com/docs/plugins#prefix-and-important (and !important, which is not relevant for your use case).

But, bear in mind that I'm an outsider, don't take this comment as actual advice of what to do since the Nuxt team could have a different answer. 👍🏽😉🤗

@BioPhoton @manniL
Apply suggestions from code review
207066f 
Co-authored-by: Alexander Lichter <github@lichter.io>
@manniL
Copy link
Member

manniL commented Jan 11, 2024

Okay, let's see how we do that:

Tailwind does not provide utils for contain (yet, sent a PR tailwindlabs/tailwindcss#12747) and also not for content-visibility (not planned so far because of browser support I suppose 🤷). Thus, the easiest would be creating plugins for both in the tailwind config. An example is already present in the tailwind docs for content-visibility and feel free to copy from my PR linked above.
These changes have to go into the tailwind.config.ts.

For contain-intrisic-* (e.g. contain-intrinsic-size), a plugin would be neat too but also a little bit more work as the utils wouldn't be static. Thus, arbitrary values in Tailwind would be the easiest. See the docs + this playground as example.

image

For the "more specific CSS" - I wonder if we either could fix this straight away in the UI library.
If you want to set it globally though, it might make sense to add an own assets/css/tailwind.css and define global styles there 👍

img, svg, span[class^="i-ph"] {
  contain:content;
}
span[class^="i-ph"] {
  content-visibility: auto;
  contain-intrinsic-size: 20px;
}

For pages/modules/index.vue, it would work the same 👍

Let me know if you have any questions

@BioPhoton
Copy link
Author

Excellent description! 😍 Helps a lot.

Thx for your time! 🙏
Will see what I can do...

DraftProducts
DraftProducts reviewed Jan 31, 2024
View reviewed changes
pages/index.vue
Comment on lines +27 to +32
.contain-intrinsic-size-500 {
contain-intrinsic-size: 200px;
}
.contain-intrinsic-size-800 {
contain-intrinsic-size: 200px;
}
Copy link

@DraftProducts DraftProducts Jan 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that correct ?

Suggested change
.contain-intrinsic-size-500 {
contain-intrinsic-size: 200px;
}
.contain-intrinsic-size-800 {
contain-intrinsic-size: 200px;
}
.contain-intrinsic-size-500 {
contain-intrinsic-size: 500px;
}
.contain-intrinsic-size-800 {
contain-intrinsic-size: 800px;
}

@danielroe
Copy link
Member

Any thoughts @benjamincanac?

@benjamincanac
Copy link
Member

I'm not familiar with those CSS properties so I won't be much of a help but I agree with @manniL's review to improve how those properties are defined using a plugin / arbitrary values.

@BioPhoton Do you plan to make the changes?

@BioPhoton
Copy link
Author

Hi just saw it. I'll push what I have tomorrow afternoon latest 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DraftProducts DraftProducts DraftProducts left review comments

@gangsthub gangsthub gangsthub left review comments

@manniL manniL manniL left review comments

@benjamincanac benjamincanac Awaiting requested review from benjamincanac

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@BioPhoton @manniL @danielroe @benjamincanac @gangsthub @DraftProducts